Skip to content

Support trust gvfs idx files for scalar prefetch #766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

tyrielv
Copy link

@tyrielv tyrielv commented Jun 11, 2025

GVFS protocol's prefetch operation (used to fetch pack files for commits and trees) includes idx files as well as pack files. Gvfs.exe (from VFSForGit) trusts the idx files it fetches, but scalar currently does not and always reindexes them locally.
This has an observed performance impact where scalar clone takes up to 10x as long as gvfs clone for large repositories.

This pull request adds support for trusting idx files to the prefetch operation in scalar, with the following behavior in priority order:

  1. If --[no-]trust-idx is specified, use that
  2. If global config gvfs.trustidxfiles is set, use that
  3. If the url to clone/prefetch is Azure DevOps, trust it
  4. Otherwise, do not trust it

@tyrielv tyrielv force-pushed the gvfs-helper-trust-pack-index branch 5 times, most recently from c41a01f to e8b8eaf Compare June 11, 2025 20:26
GVFS protocol's prefetch operation (used to fetch pack files for commits
and trees) includes idx files as well as pack files. Gvfs.exe (from
VFSForGit) trusts the idx files it fetches, but scalar currently does
not and always reindexes them locally.
This has an observed performance impact where scalar clone takes up to
10x as long as gvfs clone for large repositories.

This pull request adds support for trusting idx files to the prefetch
operation in scalar, with the following behavior in priority order:
1. If --[no-]trust-idx is specified, use that
2. If global config `gvfs.trustidxfiles` is set, use that
3. If the url to clone/prefetch is Azure DevOps, trust it
4. Otherwise, do not trust it
@tyrielv tyrielv force-pushed the gvfs-helper-trust-pack-index branch from e8b8eaf to 5cba6be Compare June 11, 2025 20:32
@tyrielv tyrielv marked this pull request as ready for review June 12, 2025 17:34
Copy link

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry that you spent time on this, but trusting the .idx files is a serious security issue. If we "trust" the .idx file, then we could have a rogue cache server deliver us a bad .idx file where it says "this Object Id has this content" but that content doesn't actually hash to that object ID.

There are similar checks when asking for a loose object by object ID: we used to trust the cache server for performance reasons, but this security boundary is really really important.

@tyrielv tyrielv closed this Jun 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants